Conversation
benesjan
force-pushed
the
07-10-fix_bug_in_pxe_getnotes
branch
from
5bc4868 to
44d5753
Compare
| const deployed = deployNullifiers[nullifier.toString()]; | ||
| const note = deployed | ||
| ? (await pxe.getNotes({ siloedNullifier: nullifier, contractAddress: deployed }))[0] | ||
| : undefined; |
There was a problem hiding this comment.
deployed is sometimes randomly undefined so had to do this here
|
Maybe this API shouldn't even exist in the first place, WDYT? |
@Thunkar I was thinking about this and generally I agree as this forces the dev to re-interpret note's content out of Noir which is bug prone. But I think there are some edge cases where a utility function returning you the notes is too cumbersome and that is when you expect to be working with a lot of notes. In that case Noir's lack of dynamic arrays would force you to paginate and that seems quite annoying. I am just working on the Closed Set Orderbook and there I can imagine this happening in prod because you would have some kind of off-chain solver that would be matching the orders. There you would probably be working with thousands of notes. This might be kind of a pie-in-the-sky usecase at this point but anyway would just keep it around for this reason. (Also is useful for debugging) But I have this feeling that @nventuro might disagree with me here 😆 |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
2 participants
PXE::getNotesdidn't trigger private state sync.This had an impact on theNotesFilteras now the contract address is mandatory as it's needed for the note sync.PXE::getPrivateEventsbehaves the same way.